feat: Generalize metrics and add nullability delta metric#43
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 11 14 +3
Lines 1018 1052 +34
=========================================
+ Hits 1018 1052 +34 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR extends Diffly’s per-column summary metrics to support non-numeric columns via an explicit Metric(fn, selector=...) wrapper, and introduces a built-in metric for nullability changes so users can quickly see how null fractions evolve between left/right frames.
Changes:
- Allow
summary(metrics=...)/ testing helpers to accept either a bare metric callable (numeric-only) or aMetricwith a Polars selector (any column types). - Add the
null_fraction_changepreset and expose it asΔNull%inDEFAULT_METRICSand the CLI. - Update summary formatting + docs, and add tests + generated summary fixtures for the new metric behavior.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| diffly/metrics.py | Adds Metric wrapper type, null-fraction change preset, and updates DEFAULT_METRICS to include selector-based metrics. |
| diffly/comparison.py | Accepts Metric objects in summary(metrics=...) and resolves bare callables as numeric-only. |
| diffly/summary.py | Allows metrics to render string results in column summaries. |
| diffly/testing.py | Broadens metrics typing/docs to allow Metric values in assertion helpers. |
| diffly/cli.py | Updates CLI help text and passes selector-based preset metrics through to summary(). |
| docs/api/metrics.rst | Documents selector-based metrics and adds Metric + null_fraction_change to the API docs. |
| tests/test_metrics.py | Adds unit tests for null_fraction_change output (including non-numeric columns). |
| tests/cli/test_cli.py | Adds CLI coverage for selecting the ΔNull% metric preset. |
| tests/summary/fixtures/metrics_null_fraction/test_metrics_null_fraction.py | New fixture generator test for summaries including ΔNull% and a selector-based custom metric. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_True_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_False_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_True_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_False_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_False_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_True_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_False_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_True_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_True_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_False_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_False_top_False_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_True_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_False_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_True_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_True_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_False_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_True_top_False_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_True_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_False_sample_rows_True_sample_pk_True.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_True_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_True_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_False_sample_rows_True_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_False_perfect_False_top_False_slim_False_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
| tests/summary/fixtures/metrics_null_fraction/gen/pretty_True_perfect_True_top_True_slim_True_sample_rows_False_sample_pk_False.txt | Generated summary fixture variant for ΔNull%. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
EgeKaraismailogluQC
left a comment
There was a problem hiding this comment.
Thank you Moritz Potthoff (@MoritzPotthoffQC), this was pleasant to review 😄 I only have some cosmetic suggestions, but also one question regarding the definition of the Null% metric:
The current definition compares the mean Null%s to each other. An alternative definition would be to check this on a per-row basis, i.e. to check how often a row that was Null is now not Null, and vice versa. When implementing the metrics feature, I rather had this second variant in mind since it makes use of the matching between each pair of rows, but the variant you implemented is of course also legitimate. My question: is this alternative definition potentially also interesting for your use case, or are you only interested in the variant which you implemented in this PR?
Co-authored-by: EgeKaraismailogluQC <128645043+EgeKaraismailogluQC@users.noreply.github.com>
…_fraction.py Co-authored-by: EgeKaraismailogluQC <128645043+EgeKaraismailogluQC@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
diffly/metrics/change.py:73
- Renaming
DEFAULT_METRICStoDEFAULT_CHANGE_METRICSindiffly.metrics.changeis a backwards-incompatible API change for consumers importing the old name. Consider keepingDEFAULT_METRICSas an alias to avoid breaking downstream code.
|
Waiting for this discussion to be settled before merging |
Motivation
I like the concept of metrics to give the user as much information about their data at first glance. I think it would be nice to extend it even further by generalizing metrics to non-numeric columns and provide metrics that describe the two datasets, not only the change. The specific use-case I have for this is to explain how the share of null values in a column changed.
Changes
metricssubpackagechangesubmoduledatasubmodule for the new metric type (not only numeric)<old> -> <new> (<delta>).